Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for app shell navigation via AMP Shadow DOM #1519

Open
wants to merge 86 commits into
base: develop
Choose a base branch
from

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Oct 22, 2018

Note: This is somewhat of a prototype for what could be supported in AMP itself as seamless page transitions: ampproject/amphtml#12981 / ampproject/amphtml#14738


For a theme to support app shell via the AMP plugin, the bare minimum that needs to be done is:

  1. Activate on PWA plugin.
  2. Ensure your theme (and plugins) work entirely in the AMP plugin's Transitional mode. You must use Transitional mode as opposed to Standard mode for now. Make sure that you have “Serve all templates as AMP regardless of what is being queried” enabled.
  3. Identify the element that contains the markup that changes as you navigate from page to page and wrap it with two function calls to indicate where the app shell container is:
<?php amp_start_app_shell_content(); ?>
    <?php get_template_part( 'content' ); ?>
<?php amp_end_app_shell_content(); ?>
  1. Opt-in to AMP theme support for app shell by adding the following to your functions.php:
add_theme_support( 'amp', array(
    'paired' => true,
    'app_shell' => true,
) );
  1. You should define an offline.php template in your theme.
  2. [Deprecated] You should mark for precaching any scripts and stylesheets needed by the app shell. This can be done via:
wp_style_add_data( 'foo-special-colors', 'precache', true );
wp_script_add_data( 'bar-special-behavior', 'precache', true );

For theme which adds support for AMP app shell see https://github.com/xwp/twentyseventeen-westonson. Note that this theme copies some code from the Twenty Seventeen parent theme in order to add make the necessary modifications. For example, the js/global.js file modified as per this diff. The theme has app shell behind a theme mod flag, so make sure you do wp theme mod set service_worker_navigation amp_app_shell via WP-CLI. See also how it adds skeleton content to the app shell. See also plugin to enable stale-while-revalidate caching strategy for navigation requests.

This was presented at CDS 2018; see related portion of talk.

For a demo of the app shell theme in action, see https://dev-showcase-pwa.pantheonsite.io/

See also this short screen cast: https://youtu.be/oAiIbhHdoXM

For some more background on this, see GoogleChromeLabs/pwa-wp#12 (comment)

Todo

  • Identify the element that contains the content
  • Add a query var which is used to obtain the document without the content being populated; add another query var which is used to omit the content for the shell response. Make sure this is done before sanitizers run.
  • Preserve admin bar in shadow DOM.
  • Add dedicated element which contains the content regardless of the id.
  • Ensure all style and link elements get preserved during content isolation.
  • Add Shadow DOM JS in outer shell request.
  • Let shadow root element be direct child of content element. This avoids styling problems.
  • Synchronize title.
  • Synchronize body classes.
  • Synchronize nav menu item classes.
  • Add JS which is responsible for history management, navigation, fetching, and invoking Shadow DOM API. Intercept the clicks on links in the shell and in the shadow to then load the new shadow doc.
  • Bootstrap spa onto frontend, automatically swap out content with shadow DOM AMP doc when navigating to another page? This handles case of smooth transition when navigating after landing on a site for the first time before the service worker is installed. Afterward, the app she would be served instead.
  • Fix lack of trailing slash causes redirect which drops the required app shell query param.
  • Trigger an event on the document when a page is transitioned.
  • Transfer admin bar from shadow to shell document.
  • Use AMP Shadow streaming API.
  • Update the class name for the menu item the instant that it is clicked.
  • Flag that the deprecation of the non-streaming API would break this.
  • Use async external script. Include in precache.
  • When declaring theme support, include the script handles to print in addition to shadow API script.
  • Make sure that the AMP runtime does not conflict with our custom script which intercept clicks.
  • When content is extracted in shell, make sure that shell serves skeleton initially.
  • Make sure app shell has revision. Vary cache by navigation menus, theme mods, etc.
  • Figure out injecting into service worker for navigation. Disable preload and then override entirely the URL that is served.
  • Make sure app shell is precached, as well as offline and error inners
  • Implement two methods of integrating app shell: bootstrapping app shell onto SW-unmanaged page, and another where app shell loads the content from the start.
  • Synchronize meta (link, meta, JSON-LD, etc)
  • Omit admin bar from shell? Or rather, prevent app shell entirely if user is logged-in?
  • Allow shadow doc request to respond with 400 if amp is disabled and so then cause navigation at top? No, just do not follow redirects from ?amp to not-?amp. If such a redirect happens, navigate the entire page.
  • Stale while revalidate will be able to update. This can be done even on streaming by leaving script in the page which listens for broadcast update.
  • Make sure that the app shell serves the appropriate initial body class based on whether or not the homepage is initially requested.
  • Preserve amp-analytics in shadow DOM; analytics needs to be excluded from app shell.
  • When requesting app shell contents force accepting sanitization
  • How can we handle a case where a user navigate to a URL that has amp disabled. Add a fury fragment to indicate that the service worker should bypass the use of the app shell
  • Scroll to content of it is out of view on mobile.
  • Prevent navigation when # anchor link.
  • Add support for navigating to anchors on other URLs.
  • Download web components API JS to serve locally.

@westonruter
Copy link
Member Author

Note that travis is failing due to Gutenberg being merged into core.

@westonruter westonruter force-pushed the add/spa-amp-shadow-dom branch from e2df4ed to 039ef47 Compare October 22, 2018 23:57
@westonruter westonruter force-pushed the add/spa-amp-shadow-dom branch from 20914dd to 84a2749 Compare October 28, 2018 20:48
@westonruter westonruter added the Needs Issue Label for PRs that do not have an corresponding issue label Apr 16, 2020
@westonruter westonruter changed the title Add support for app shell navigation via Shadow DOM Add support for app shell navigation via AMP Shadow DOM Apr 28, 2020
includes/amp-helper-functions.php Outdated Show resolved Hide resolved
includes/class-amp-theme-support.php Outdated Show resolved Hide resolved
includes/class-amp-theme-support.php Outdated Show resolved Hide resolved
westonruter and others added 7 commits July 20, 2020 16:04
Co-authored-by: Piotr Delawski <[email protected]>
Building the app shell JS with webpack allows us to use third-party code
(e.g. @WordPress packages) easily. We also take advantage of the code
minification and babel transpilation.

This is a step towards using @wordpress/hooks package in the app shell
script.
The change to the inner component URL format introduced in 6054285
broke the app shell experience. While URL format (use an URL segment
instead of a query parameter) was changed in the JS implementation, the
back-end PHP handler was still expecting the old format. It resulted in
loading 404 pages instead of actual inner components for requested pages.

The follow up commit will allow altering of the URL format if a plugin
integrator really needs to do so.
In some cases, the plugin integrator may need to alter the format of the
inner component URL.

While it is possible to change the way incoming requests for inner
components are handled on the back-end (PHP), there was no way to change
the URL format of the request on the front-end (JS).

This commit makes the inner component request URL filterable using
wp.hooks. Here's how you could filter the URL in your plugin or theme
so that instead of using a query parameter, a new segment is added to
the URL:

```
if ( ampAppShell && ampAppShell.hooks ) {
	function filterInnerComponentUrl( url, baseUrl, componentQueryVar ) {
		const filteredUrl = new URL( baseUrl );
		const pathSuffix = '_' + componentQueryVar + '_inner';
		filteredUrl.pathname = filteredUrl.pathname + pathSuffix;

		return filteredUrl;
	}

	ampAppShell.hooks.addFilter( 'amp.appShell.innerComponentUrl', 'ampWpAppShell/filterInnerComponentUrl', filterInnerComponentUrl );
}
```
Allow filtering of inner component URL
delawski and others added 5 commits July 27, 2020 17:07
* develop: (2551 commits)
  Update dependency eslint-plugin-jest to v23.18.2
  Update dependency eslint-plugin-jest to v23.18.1
  Fix TikTok test
  Fix test failure in WP 4.9
  Add tests for ObsoleteBlockAttributeRemover
  Skip adding data attributes for gallery shortcodes
  Revert block deprecation logic
  Harden pattern to match block comment and start tag
  Improve phpdoc for ObsoleteBlockAttributeRemover
  Improve logic for obtaining obsolete attribute pattern
  Introduce ObsoleteBlockAttributeRemover service
  Update dependency webpack to v4.44.0
  Modify the block attributes during its deprecation
  Use DOMElement as return type for HasCaption::get_caption_element()
  Prevent removal of data attributes in sanitize_raw_embeds and fix boolean checks
  Ensure data-amp-carousel=true attribute is present for proper styling of gallery block
  Supply missing attribute value for gallery shortcode output
  Ensure comments_popup_link() is also filtered to prevent mobile redirection
  Test AMP_Theme_Support::amend_comments_link
  Remove unused imports
  ...

# Conflicts:
#	amp.php
#	includes/amp-helper-functions.php
#	includes/class-amp-theme-support.php
#	includes/sanitizers/class-amp-style-sanitizer.php
#	includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
Merge develop and resolve conflicts
@github-actions
Copy link
Contributor

Plugin builds for 77fca7d are ready 🛎️!

delawski added a commit to xwp/amp-app-shell that referenced this pull request Aug 5, 2020
This is a first step of a migration of an app shell experimental
implementation proposed in ampproject/amp-wp#1519
delawski added a commit to xwp/amp-app-shell that referenced this pull request Aug 6, 2020
delawski added a commit to xwp/amp-wp that referenced this pull request Aug 6, 2020
delawski added a commit to xwp/amp-wp that referenced this pull request Aug 6, 2020
delawski added a commit to xwp/amp-app-shell that referenced this pull request Aug 13, 2020
In the original app shell implementation proposed in ampproject/amp-wp#1519
the is_amp_endpoint() function was modified in order to return true if
an inner component was requested.

In order to achieve the same effect without altering the original is_amp_endpoint()
implementation, we're explicitly asking for an AMP document when requesting
the inner component. Simply, an AMP slug is appended to a URL when fetching
a new page.
delawski added a commit to xwp/amp-app-shell that referenced this pull request Aug 14, 2020
@schlessera schlessera added the WS:Core Work stream for Plugin core label Aug 18, 2020
@CLAassistant
Copy link

CLAassistant commented Nov 4, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 6 committers have signed the CLA.

✅ westonruter
✅ mehigh
✅ dero
✅ delawski
❌ miina
❌ indieisaconcept
You have signed the CLA already but the status is still pending? Let us recheck it.

@westonruter westonruter mentioned this pull request Jul 21, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: no Has not signed the Google CLA Integration: PWA Needs Issue Label for PRs that do not have an corresponding issue WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants